Skip to content

fix(driver-sql): schema drift reports a single-value JSON-class field on a stale text column - #16073

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-15771-schema-drift-single-value-json-column
Sep 6, 2026
Merged

fix(driver-sql): schema drift reports a single-value JSON-class field on a stale text column#16073
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-15771-schema-drift-single-value-json-column

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Fixes #15771

The defect was a FORK, not a missing rule

The driver decides a column's type with one predicate and reported drift about it with another:

sql-driver.ts:16085   col = JSON_COLUMN_TYPES.has(type) ? this.jsonColumn(table, name) : table.string(name);
sql-driver.ts:16301   return JSON_COLUMN_TYPES.has(type) || !!field.multiple;   the WRITER
schema-drift.ts:944   const declaresJsonColumn = field.multiple === true;       the DETECTOR

So a SINGLE-value JSON-class field — file, image, avatar, video, audio, location, address, composite, record, vector, json, and the option families — sitting on a varchar/text column was written as JSON by the writer and did not exist to the differ. syncSchema is additive and never migrates a column's type, so the divergence was permanent, and nothing else reports it, so it was silent.

This PR deletes the fork: the detector now reads the writer's own predicate. No new criterion was invented.

Reproduction, before and after

One diffManagedTable call per type, character varying(2048) column, both enforcing dialects:

BEFORE                                                   AFTER
single-value file     / postgres : findings=0            findings=1  manual_column_type_change
single-value file     / mysql    : findings=0            findings=1  manual_column_type_change
single-value location / postgres : findings=0            findings=1  manual_column_type_change
single-value location / mysql    : findings=0            findings=1  manual_column_type_change
multi-value file      / postgres : findings=1  (control) findings=1  (control, unchanged)
JSON-class spec types written as JSON but NOT detected: 15 of 15   ->  0 of 15

The multiple: true row is the firing control in the same run: the differ was working, and this whole population was invisible to it.

The message SPLITS, because the shipped remedy is wrong for half of it

os migrate multi-value-columns repairs a stale column by WRAPPING each stored value in a one-element JSON array (json_build_array(col) on Postgres, JSON_ARRAY(col) on MySQL). That is right for a field whose value is an array and wrong for one whose value is a scalar or an object. Measured with the command's own planner, on a single-value finding carrying the multi-value message:

targets: 1  refusals: 0   dialect: postgres
  ALTER TABLE "proj_task" ALTER COLUMN "doc" TYPE json USING (CASE ... ELSE json_build_array("doc") END)

And measured on an in-memory SQLite cell, what that would do to the value:

ok      : value="file_01HXYZ"          typeof=string  isArray=false     what ObjectStack stores
wrapped : value=["\"file_01HXYZ\""]    typeof=object  isArray=true      after the array remedy

So both populations are REPORTED and only one is offered the remedy:

  • Array-valued (multiple: true, plus the inherently-multi option types, whose value is a list with or without the flag) keeps its message character for character. That is a contract, not a style choice: planStaleColumnTargets recovers the dialect by testing message.includes(manualJsonConversionSql(d, table, column)), so a reworded message makes the command refuse every finding. Proven against origin/main itself while writing the change — the whole entry, message included, compared equal for lookup, string and file with multiple: true, on both dialects.
  • Single-value carries a message of its own that names neither the command nor the statement, explains why the automated route is withheld, and describes the by-hand conversion. The same probe therefore recovers no dialect and the command REFUSES the entry with remedy_not_recognized — its designed branch for a message it cannot read — instead of running array SQL over scalar rows. Pinned from both sides.

⛔ No new remedy DDL was invented. manualJsonConversionSql's two existing forms are EXECUTED against live servers by the suite, and two of their arms were corrected only by running an earlier version against a live database; there is no live Postgres or MySQL cell in this container, so a third unmeasured statement is not something this PR ships. The remedy split is filed as #16072 with the three options and what each costs.

SQLite: the reverse control, re-measured

On SQLite the harm is not reachable, and that is a measurement rather than a scoping convenience. On an in-memory cell, a single-value file field over the stale varchar(2048) column and over the driver's own column:

stale column type : varchar        fresh column type : text
stale RAW stored  : "file_01HXYZ"  fresh RAW stored  : "file_01HXYZ"     byte-identical
stale read-back   : "file_01HXYZ"  fresh read-back   : "file_01HXYZ"     identical
sqlite detectManagedDrift manual_column_type_change: []

The read path parses a textual column regardless of what it calls itself, so there is nothing to corrupt and nothing to report. The pin keeps that leg as its reverse control: without it, every positive assertion above is equally satisfied by a detector that always reports.

What the consequence chain is, and is NOT

The card's downstream chain is REASONED FROM CODE and is not upgraded here: formatInput JSON.stringifys every isJsonField value unconditionally, and the read-side JSON.parse lives only inside the isSqlite arm of formatOutput, so on Postgres/MySQL a varchar column returns the quoted text verbatim, isFileIdToken refuses it, the read resolver never expands it and the ownership hooks never claim it. There is no live Postgres or MySQL cell in this container, so that chain is stated exactly as the card stated it. What IS measured here is the stored form (SQLite, byte-for-byte), the detector's before/after verdicts, and the planner's behaviour.

Also corrected by the same predicate: a single-value JSON-class field declaring a maxLength over a wider varchar column used to be reported as narrow_varchar at category destructive, inviting os migrate apply --allow-destructive to rewrite the column to a narrower varchar — the opposite of the repair it needs. It is now reported once, as the base-type divergence. Pinned.

Scope

packages/cli/src/commands/generate.ts is untouched: the VARCHAR(2048)-vs-json-column divergence in the generated migration is #15041's subject and that file's own docblock already acknowledges it. This change is only about the detector, and the blind spot it closes applies to every single-value member of the writer's set whichever way #15041 goes. Out of scope here, still open: #15041, and #16072 for the remedy split.

Published surface — the dist/index.d.ts ablation

Built at head, swapped schema-drift.ts back to origin/main, rebuilt, diffed the built dist/index.d.ts, restored byte-exact:

ON-DISK PROOF  new-symbol hits=0 (expect 0) · reverted-line hits=1 (expect 1) · blob differs
dist preflight (--absent) exit=0        the mutation reached dist/
ABLATED driver-sql pins exit=1          9 failed | 22 passed | 3 skipped
ABLATED cli pin       exit=1            1 failed | 11 passed
RESTORE PROOF  blob == HEAD blob · git diff HEAD empty
dist preflight (present) exit=0         marker present in 2 built files
RESTORED driver-sql pins exit=0         31 passed | 3 skipped
RESTORED cli pin      exit=0            12 passed
dist/index.d.ts diff (origin/main build vs head build): 0 changed lines

Both limbs of the surface question, stated separately:

  • Type/export surface: unchanged. The built dist/index.d.ts is byte-identical between the two builds. JSON_COLUMN_FIELD_TYPES is module-exported so this package's own suites can pin it and is deliberately NOT added to index.ts — the same call UNBOUNDED_TEXT_FIELD_TYPES and MULTI_VALUE_COLUMN_REMEDY_COMMAND make.
  • Accept/reject behaviour: nothing is newly accepted or rejected. This widens a DIAGNOSTIC VERDICT's population. No request is admitted or refused, no write is validated, no published accept-set moves. The one real downstream consequence — os migrate multi-value-columns now meets a population it never met — is handled by routing that population into the command's refusal branch, so nothing new is executed there either.

Tests

New: packages/drivers/driver-sql/src/schema-drift.json-column-parity.test.ts holds the differ's JSON-class predicate equal to the writer's isJsonField over every FieldType the spec declares, in both directions, and compares the two halves where they actually meet — through diffManagedTable's output rather than between two constants. Extended: schema-drift.base-type-mismatch.test.ts (the single-value cases on both enforcing dialects, the SQLite non-finding as the reverse control, the whole-vocabulary sweep, the message split in both directions, the byte-identity of the array message, and the narrow_varchar regression) and packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts (the refusal, with an array-valued target as the contrast in the same run).

Verified at 784956909:

pnpm --filter @objectstack/driver-sql test          155 passed | 9 skipped (164 files) · 2389 passed | 140 skipped
pnpm --filter @objectstack/cli exec vitest run (9 drift-consumer files)   9 passed · 84 passed
pnpm --filter @objectstack/driver-sql typecheck     exit 0
pnpm --filter @objectstack/cli typecheck            exit 0  (tsc --listFiles confirms both edited test files are in the program)
gate family from `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` at 784956909:
  43 of 45 exit 0, plus `pnpm check:nul-bytes` exit 0
  check:dual-build-cjs-loads and check:i18n-coverage exit 3 = "COULD NOT MEASURE / this is NOT a pass:
  nothing was measured" — both need a full workspace build this worktree does not have. Not findings;
  CI builds the workspace and measures them there.

Generated by Claude Code

… on a stale text column

`createColumn` gives a json column to every JSON-class TYPE and `isJsonField`
is `JSON_COLUMN_TYPES.has(type) || !!field.multiple`, but `diffManagedTable`'s
base-type branch asked only `field.multiple === true`. A single-value `file` /
`location` / `record` / `vector` / `json` field on a `varchar`/`text` column was
therefore written as JSON by the writer and invisible to the differ — and the
additive sync never revisits a column, so the divergence was permanent and
silent. Measured on the previous tree: all fifteen JSON-class types the spec
declares produced zero findings on that column under `postgres` and `mysql`.

The detector now reads the writer's predicate. The remedy splits by VALUE
SHAPE: `os migrate multi-value-columns` wraps each value in a one-element JSON
array, so it stays offered to array-valued fields — whose message is unchanged
character for character, which is what lets `planStaleColumnTargets` keep
recovering the dialect from it — and is withheld from single-value ones, whose
message carries neither the command nor its statement and is therefore refused
with `remedy_not_recognized` rather than running array SQL over scalar rows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…me prose

`check:doc-authoring` refuses a `#NNNN` in a string an operator reads — it
resolves to nothing without the tracker (maintainer ruling 2026-08-12). The
anchor stays in the `//` comment beside the emission and in git history; the
message now carries the CAUSE in words instead, which is what the reader
actually needs. The pin asserts that wording rather than the id.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 31403453dda6de58d2bbed91a1448a65e2d0a1a3packageMentionDocs.

Which tree this was computed on

This run read content/docs from 47738a4b4853f145ab338e2fb62ff0c7a6206062 — the merge of head 78495690974af0bf6763421241a541322fc51aa5 into base 31403453dda6de58d2bbed91a1448a65e2d0a1a3, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 47738a4b4853f145ab338e2fb62ff0c7a6206062 && git checkout 47738a4b4853f145ab338e2fb62ff0c7a6206062
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 31403453dda6de58d2bbed91a1448a65e2d0a1a3 78495690974af0bf6763421241a541322fc51aa5 && git checkout -B drift-repro 31403453dda6de58d2bbed91a1448a65e2d0a1a3 && git merge --no-ff 78495690974af0bf6763421241a541322fc51aa5

node scripts/docs-audit/affected-docs.mjs --json 31403453dda6de58d2bbed91a1448a65e2d0a1a3

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants